fix(UI): fix TreeItem expansion bug#1410
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a TreeItem expansion bug by refactoring the click handling mechanism from using individual label click handlers to a centralized focus-based approach using the MUI TreeView API reference.
- Replaced custom
onLabelClickprop handling with theonItemFocusevent handler - Simplified the label component by removing the redundant click handling logic
- Integrated MUI's TreeView API reference for better item selection management
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| RepoTree.tsx | Added API reference and centralized focus handler to replace distributed click handling |
| CustomTreeItem.tsx | Removed onLabelClick prop and related click handling logic |
| CustomLabel.tsx | Simplified component by removing onClick prop and replacing TreeItemLabel with div |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const item = apiRef.current!.getItem(itemId) | ||
|
|
||
| if (item.content_type) { | ||
| handleLabelClick(item.path, item.content_type === 'directory'); | ||
| apiRef.current?.setItemSelection({ |
There was a problem hiding this comment.
Using non-null assertion operator (!) is risky here. Consider adding a null check to handle cases where the API reference might not be initialized or the item doesn't exist.
| const item = apiRef.current!.getItem(itemId) | |
| if (item.content_type) { | |
| handleLabelClick(item.path, item.content_type === 'directory'); | |
| apiRef.current?.setItemSelection({ | |
| if (!apiRef.current) { | |
| // Optionally, you could log or handle the error here | |
| return; | |
| } | |
| const item = apiRef.current.getItem(itemId); | |
| if (item && item.content_type) { | |
| handleLabelClick(item.path, item.content_type === 'directory'); | |
| apiRef.current.setItemSelection({ |
| <div | ||
| style={{fontSize: '14px', cursor: 'pointer'}} | ||
| > | ||
| {children} | ||
| </TreeItemLabel> | ||
| </div> |
There was a problem hiding this comment.
Replacing TreeItemLabel with a div loses semantic meaning and accessibility features. Consider keeping TreeItemLabel and removing only the onClick handler instead of replacing the entire component.
#1389